Fix: correctly pipe keyword arguments to plot_dendrogram#65
Fix: correctly pipe keyword arguments to plot_dendrogram#65mstimberg merged 6 commits intobrian-team:masterfrom
Conversation
|
Hi @Utkarsha2811, thanks for the PR, passing on the keywords makes indeed sense. Could you please include this feature in the tests? Also, I think the documentation needs to state that the arguments are given to |
|
Hey @mstimberg, sure will check, I'll push the changes and update the doc. |
Utkarsha2811
force-pushed
the
fix-dendrogram-kwargs
branch
from
be5a2a2 to
8f8b536
Compare
|
Hey @mstimberg, I’ve updated the PR based on the review comments and also revised the PR description to reflect the changes more clearly. Please take a look when you get a chance and let me know what you think. though the build failure is being fixed in Once it is merged, I'll rebase this PR (then build should pass). |
mstimberg
left a comment
mstimberg
left a comment
There was a problem hiding this comment.
Thanks for the update, two more small comments below.
| # Plot the dendogram with lengths of the vertical lines representing the | ||
| # total distance to the root | ||
| plt.plot(x_values[0], length_metric[0], 'ko', clip_on=False) | ||
| plt.plot(x_values[0], length_metric[0], marker='o', clip_on=False, **kwds) |
There was a problem hiding this comment.
It will rarely occur, but it would be cleaner if we dealt with the situation that the user provides clip_on or lw (see below) as keyword arguments themselves, which currently breaks with a "keyword argument repeated" error.
There was a problem hiding this comment.
Thanks, Addressed
| The `~brian2tools.plotting.morphology.plot_dendrogram` function does the same thing, but in contrast to the other | ||
| plot functions it does not allow any customization at the moment, so there is no benefit over using | ||
| `~brian2tools.plotting.base.brian_plot`. | ||
| The `~brian2tools.plotting.morphology.plot_dendrogram` function does the same thing, but also accepts additional |
There was a problem hiding this comment.
The previous text is still correct: plot_dendrogram does not add any functionality over brian_plot, since you can also give the keyword arguments to brian_plot. It would still be nice to give an example for the keyword arguments, but 1) stating that this is possible both with brian_plot and plot_dendrogram, and 2) including an example image in the documentation.
There was a problem hiding this comment.
yes, updated the doc
|
Thanks for the update @Utkarsha2811. The file |
Thanks for reviewing @mstimberg, I missed generating plot image, Added! |
2 participants
Fix: Pass styling keyword arguments through plot_dendrogram
Problem
When calling plot_dendrogram (or brian_plot on a morphology), any styling arguments like color='red' or alpha=0.5 were silently ignored. There was no way to customize the appearance of the dendrogram tree.
Solution
Added **kwds support to plot_dendrogram so that styling arguments are now correctly forwarded to the three underlying matplotlib calls: plot, vlines, and hlines. This means users can now do:
plot_dendrogram(morpho, color='red', alpha=0.7, linewidth=2)
brian_plot(morpho, color='blue')
Changes
• brian2tools/plotting/morphology.py — plot_dendrogram now accepts **kwds and passes them to all three matplotlib drawing calls. Updated the docstring to clearly state that kwargs go to plot, vlines, and hlines, and that only arguments compatible with all three (e.g. color, alpha, linewidth) should be used.
• brian2tools/tests/test_plotting.py — Added test cases verifying that plot_dendrogram works correctly with color, alpha, and linewidth kwargs, and that these kwargs also flow through correctly when using brian_plot.
• docs_sphinx/user/plotting.rst — Removed the outdated note saying plot_dendrogram "does not allow any customization at the moment". Replaced it with an accurate description of the new kwargs support, including a usage example.